Skip to content

Fix source blob backup ETag comparison failing every blob - #1273

Merged
Paul Lizer (paullizer) merged 3 commits into
Developmentfrom
fix/backup-source-blob-etag
Aug 18, 2026
Merged

Fix source blob backup ETag comparison failing every blob#1273
Paul Lizer (paullizer) merged 3 commits into
Developmentfrom
fix/backup-source-blob-etag

Conversation

@paullizer

Copy link
Copy Markdown
Contributor

Fixes #1271

Problem

Every source blob in every Data Management backup failed with Source blob changed while it was being backed up. — 100% failure rate, zero retries, across all containers. User documents, group documents, public documents, and chat attachments have never been backed up. The job still reported completed_with_warnings, which made it easy to miss.

From job data_management_partial_20260818T0300Z:

Container Blobs read Copied Failed
user-documents 71 0 71
group-documents 427 0 427
public-documents 95 0 95
personal-chat 19,394 0 19,394
[DATA_MANAGEMENT] Source blob backup completed with failures. -- {'container': 'personal-chat',
 'failed_count': 19394, 'copied_count': 0, 'source_read_count': 19394,
 'failure_reasons': '19394x Source blob changed while it was being backed up.'}

Root cause

_transfer_backup_source_blob compares two ETags that come from different Azure SDK code paths with different transport formats:

Value Origin SDK path Format
source_etag list_blobs() get_blob_properties_from_generated_code() reads the XML <Etag> element 0x8DE...
current_source_etag get_blob_properties() BlobProperties(**headers) reads the HTTP ETag header "0x8DE..."

The HTTP ETag header is an RFC 7232 quoted-string; the List Blobs XML element is not. azure-storage-blob==12.24.1 normalizes neither — I grepped the installed package to confirm. So the check was always 0x8DE... != "0x8DE...".

RuntimeError is not retryable under _is_retryable_backup_blob_error, so every blob failed on the first attempt — matching the observed Retries / throttles: 0 / 0.

Independent confirmation the blobs did not actually change: ranged reads send that same unquoted ETag as an If-Match precondition and Azure accepted them. Only the Python-side comparison was wrong. The guard also runs after staging and commit, so every blob was fully downloaded, encrypted, and uploaded before being discarded — and left behind as an orphaned pending artifact.

Changes

1. ETag normalization — new _normalize_backup_etag strips transport quoting and the optional W/ weak-validator prefix, applied only at the comparison site.

source_item["source_etag"] deliberately keeps the exact value returned by list_blobs(), so the If-Match precondition sent to Azure is byte-for-byte unchanged from current production behavior. The guard is preserved — a genuine mid-transfer change still fails.

2. Checkpoint batchingrecord_transfer_result previously called persist() per item: 19,394 Cosmos writes for one container, capping throughput near 6 items/sec and stretching the run to 74 minutes. New maybe_persist checkpoints on manifest batch size (100) or a 15-second interval, whichever comes first. The job lease is still asserted every item, and the existing tail persist() still flushes the final partial batch. Worst-case re-work after an interrupted run stays bounded at 100 items or 15 seconds.

3. Functional tests that silently passed under pytest — while validating this fix I found the repo's standard test template hides failures:

def test_x():
    try:
        assert ...
        return True
    except Exception:
        return False

Returning a value instead of raising makes pytest report the test as passed with only a PytestReturnNotNoneWarning. A deliberately broken build reported 7 passed while the transfer was genuinely failing. Both backup test files now assert directly and return None, with the __main__ block preserving standalone output and exit codes. Suite warnings dropped 13 → 1.

Scope check

The migration path (_copy_source_blobs_to_target) has a visually identical comparison, but sources source_properties from get_blob_properties(), so both operands are already quoted. Migration is not affected and is left unchanged.

Validation

New test functional_tests/test_data_management_backup_source_blob_etag.py drives the real _transfer_backup_source_blob against in-memory blob clients with an unquoted listing and a quoted fetch.

7 passed in 2.51s
Test Assertion
test_etag_normalization_strips_transport_quoting Quoted, unquoted, weak, padded ETags normalize identically
test_listed_and_fetched_etags_compare_equal Raw listed ETag preserved for If-Match; both formats equal once normalized
test_transfer_succeeds_across_list_and_get_etag_formats End-to-end transfer succeeds and promotes metadata to succeeded
test_genuinely_changed_source_blob_still_fails ETag changing after download is still rejected
test_verified_artifact_matches_source_version Reuse detection still keys off recorded source version
test_checkpoint_interval_is_bounded Interval and batch size within safe bounds
test_version_is_at_least_fix_version Version floor

Regression probe: neutralizing _normalize_backup_etag fails 4 of 7 tests, including the end-to-end transfer, reporting the exact production message:

Transfer must succeed, got 'failed' ('Source blob changed while it was being backed up.')

Full Data Management suite: 154 passed, 1 failed. That failure (test_backup_recovery_and_admin_progress_are_bounded_and_sanitized) was confirmed pre-existing on origin/Development in the previous PR and is unrelated.

Version

0.250.2170.250.218

Follow-up

#1272 tracks the cosmetic issue where completed jobs still display Current container: Waiting and running step badges.

Documentation

  • docs/explanation/fixes/SOURCE_BLOB_BACKUP_ETAG_FIX.md
  • docs/explanation/release_notes.md

Every source blob in every Data Management backup failed with "Source blob
changed while it was being backed up." at a 100% rate with zero retries, so
user documents, group documents, public documents, and chat attachments were
never actually backed up. The job still reported completed_with_warnings,
which made the condition easy to miss.

_transfer_backup_source_blob compared an ETag obtained from list_blobs()
against one obtained from get_blob_properties(). Azure returns those in
different transport formats: the List Blobs XML <Etag> element is unquoted
while the HTTP ETag response header is an RFC 7232 quoted-string, and
azure-storage-blob does not normalize either. The comparison was therefore
always unequal. RuntimeError is not retryable, so each blob failed on the
first attempt after already being downloaded, encrypted, and uploaded.

Normalize both operands before comparing, stripping transport quoting and the
optional W/ weak-validator prefix. source_item["source_etag"] still carries
the exact value returned by list_blobs(), so the If-Match precondition sent on
ranged reads is byte-for-byte unchanged. The guard is preserved: a genuine
mid-transfer source change still fails.

The migration path has a visually similar comparison but sources both operands
from get_blob_properties(), so it is unaffected and left unchanged.

Also included:

- Batch source blob checkpoints. record_transfer_result previously persisted
  once per item, producing 19,394 Cosmos writes for a single container and
  capping throughput near six items per second. Checkpoint on manifest batch
  size or a 15 second interval instead, still asserting the job lease per item.
- Fix functional tests that silently passed under pytest. Tests using the
  try/except and "return False" template return a value rather than raising,
  so pytest reported a broken build as 7 passed. Both backup test files now
  assert directly and return None.

Fixes #1271
@paullizer Paul Lizer (paullizer) added bug Something isn't working python Pull requests that update python code labels Aug 18, 2026
@paullizer

Copy link
Copy Markdown
Contributor Author

Copilot resolve the merge conflicts in this pull request

Co-authored-by: paullizer <34814295+paullizer@users.noreply.github.com>

Copilot AI commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in 96d7c26.

Only docs/explanation/release_notes.md conflicted. Development had meanwhile shipped its own v0.250.218 section (credential key redaction, CosmosClient import bindings, privacy logging audit), so I kept that section intact and moved this PR's three entries into a new v0.250.219 section above it.

Because Development claimed 0.250.218, this fix is renumbered to 0.250.219 in config.py, SOURCE_BLOB_BACKUP_ETAG_FIX.md, and the test header/version floor. No production logic changed in the merge — _normalize_backup_etag and the batched checkpointing are unchanged.

Verified after merging: test_data_management_backup_source_blob_etag.py is 7/7 and test_data_management_backup_cosmos_pagination.py is 4/5, the one failure being a missing azure.monitor package in this sandbox rather than a code issue.

@paullizer

Copy link
Copy Markdown
Contributor Author

Copilot resolve the merge conflicts in this pull request

Co-authored-by: paullizer <34814295+paullizer@users.noreply.github.com>

Copilot AI commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Resolved again in 1792dd9Development moved forward with #1274, which claimed v0.250.219, the number this PR took in the previous round.

Same resolution as before: only release_notes.md conflicted. I kept #1274's v0.250.219 section (agent document search citations, plus its UI enhancement entry) intact and moved this PR's three entries into a new v0.250.220 section above it. Renumbered 0.250.2190.250.220 in config.py, SOURCE_BLOB_BACKUP_ETAG_FIX.md, and the test header/version floor, leaving #1274's files at 0.250.219.

functions_data_management.py merged cleanly — the diff against Development is still only my six files, with _normalize_backup_etag and the batched checkpointing unchanged.

Validation after the merge (installed the missing test deps this time, so the previously unrunnable tests actually execute):

  • test_data_management_backup_source_blob_etag.py — 7/7
  • test_data_management_backup_cosmos_pagination.py — 5/5 (was 4/5 last round only because azure.monitor was missing)
  • test_agent_document_search_citations.py — 12/12, confirming Cite documents agents find via document search #1274's version floor still holds at 0.250.220

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working python Pull requests that update python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants